Skip to content

feat: prevent wildcard deletion in deleteUser/deleteRole APIs #519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
hsluoyz merged 5 commits into from
Jan 10, 2026
Merged

feat: prevent wildcard deletion in deleteUser/deleteRole APIs #519

hsluoyz merged 5 commits into from
Jan 10, 2026

Conversation

Copy link
Contributor

Copilot AI commented Dec 8, 2025 •
edited
Loading

Fix deleteUser() deleting all policies when called with empty string

Root Cause

The enforcer.deleteUser() method does not validate the user parameter. When called with an empty string '', it causes all policies to be deleted because empty string values are treated as wildcards in removeFilteredPolicy. This results in SQL queries like:

DELETE FROM casbin_rule WHERE ptype = 'p';
DELETE FROM casbin_rule WHERE ptype = 'g';

Changes Made

  • Identify root cause: empty string parameter acts as wildcard
  • Add validation to deleteUser() to reject empty/falsy user values
  • Add validation to deleteRole() which has the same issue
  • Add validation to deletePermission() and other similar methods
    • deleteRoleForUser() - validates both user and role
    • deleteRolesForUser() - validates user
    • deletePermissionsForUser() - validates user
    • deletePermissionForUser() - validates user
    • deletePermission() - validates permission array is not empty
  • Add tests to verify empty string handling (7 new tests)
  • Run existing tests to ensure no regressions (all 190 tests pass)
  • Fix linting issues
  • Request code review (no issues found)
  • Run security scan (no vulnerabilities found)
  • Revert yarn.lock to original state

Security Summary

No security vulnerabilities were found in the changes. The fix actually improves security by preventing accidental data loss when invalid parameters are provided.

Original prompt

This section details on the original issue you should resolve

<issue_title>enforcer.deleteUser() wiped out all data from casbin_rule table</issue_title>
<issue_description>Logged queries shows that following queries are executed when calling enforcer.deleteUser()

DELETE from casbin_rule WHERE ptype = 'p';
DELETE from casbin_rule WHERE ptype = 'g'; 
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@CLAassistant
Copy link

CLAassistant commented Dec 8, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@hsluoyz hsluoyz marked this pull request as ready for review December 8, 2025 12:03
Copilot AI changed the title [WIP] Fix enforcer.deleteUser() to prevent data loss Prevent wildcard deletion in deleteUser/deleteRole methods Dec 8, 2025
Copilot AI requested a review from hsluoyz December 8, 2025 12:23
@hsluoyz
Copy link
Member

hsluoyz commented Jan 10, 2026

@copilot don't modify yarn.lock

Copy link
Contributor Author

Copilot AI commented Jan 10, 2026

@copilot don't modify yarn.lock

Reverted yarn.lock to its original state in commit 5599e1a.

@coveralls
Copy link

coveralls commented Jan 10, 2026

Pull Request Test Coverage Report for Build 20874840964

Details

  • 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 78.472%
Totals Coverage Status
Change from base Build 20679550322: 0.2%
Covered Lines: 1661
Relevant Lines: 2017
💛 - Coveralls

@hsluoyz hsluoyz changed the title Prevent wildcard deletion in deleteUser/deleteRole methods feat: prevent wildcard deletion in deleteUser/deleteRole APIs Jan 10, 2026
@hsluoyz hsluoyz merged commit 122a00c into master Jan 10, 2026
7 of 9 checks passed
hsluoyz added a commit that referenced this pull request Jan 10, 2026
@hsluoyz
feat: prevent wildcard deletion in deleteUser/deleteRole APIs (#519)
4d68d28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hsluoyz hsluoyz Awaiting requested review from hsluoyz

Assignees

@hsluoyz hsluoyz

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

enforcer.deleteUser() wiped out all data from casbin_rule table

4 participants

@CLAassistant @hsluoyz @coveralls